refactor: don't wait ai settings#1568
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the connection creation process to make AI settings generation asynchronous, preventing it from blocking the connection creation response. The main change converts the AI scan from an awaited operation to a fire-and-forget pattern.
Changes:
- Modified connection creation to run AI database scanning in the background without blocking the response
- Updated code formatting from double quotes to single quotes throughout the modified file
- Changed transaction mode for group creation from ON to OFF (appears unrelated to PR purpose)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/src/entities/connection/use-cases/create-connection.use.case.ts | Converted AI scan operation to fire-and-forget pattern with error handling, added Sentry import, and applied code formatting changes |
| backend/src/entities/connection/connection.controller.ts | Changed transaction mode for createGroupInConnection from ON to OFF |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| }; | ||
| return await this.createGroupInConnectionUseCase.execute(inputData, InTransactionEnum.ON); | ||
| return await this.createGroupInConnectionUseCase.execute(inputData, InTransactionEnum.OFF); |
There was a problem hiding this comment.
The transaction mode for creating a group in a connection has been changed from InTransactionEnum.ON to InTransactionEnum.OFF. This change appears unrelated to the stated PR purpose of "refactor: don't wait ai settings" and could be accidental.
Removing transactions from group creation operations could lead to data inconsistency issues if the operation fails partway through. The createGroupInConnection use case creates a new group entity and saves it to the database, which should ideally be atomic.
If this change is intentional, it should be explained in the PR description or separated into a different commit with proper justification. If accidental, it should be reverted to InTransactionEnum.ON.
| await this.sharedJobsService.scanDatabaseAndCreateSettingsAndWidgetsWithAI( | ||
| connectionCopy, | ||
| ); | ||
| if (isConnectionTestedSuccessfully && !isConnectionTypeAgent(connectionCopy.type)) { |
There was a problem hiding this comment.
The fire-and-forget logic in the finally block could throw an error if connectionCopy remains null. If an exception occurs in the try block before line 87 (where connectionCopy is assigned), the finally block will execute with connectionCopy still being null, causing connectionCopy.type to throw a TypeError.
Consider adding a null check for connectionCopy before accessing its properties.
| if (isConnectionTestedSuccessfully && !isConnectionTypeAgent(connectionCopy.type)) { | |
| if (connectionCopy && isConnectionTestedSuccessfully && !isConnectionTypeAgent(connectionCopy.type)) { |
No description provided.